-
Notifications
You must be signed in to change notification settings - Fork 49
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Refactor interface tests and add more #279
Refactor interface tests and add more #279
Conversation
ed25519 and ecdsa key pairs creation script: ``` import securesystemslib.interface as i i.generate_and_write_ed25519_keypair( "tests/data/keystore/ed25519_key", password="password") i.generate_and_write_ecdsa_keypair( "tests/data/keystore/ecdsa_key", password="password") ``` Also adds json file for custom json key format parsing error testing.
- Remove repetitive test dir creation in individual test functions. Use setUp function instead - Remove redundant test dir creation in setUpClass function. - Remove repetitive key generation and key import code used to test one or the other "unit". Instead aggregate key generation and loading testing in a single test function per key type (rsa, ed25519, ecdsa). - Use context manager for assertRaises to increase readability - Use test tables (for loops) for more extensive testing of different combinations of args and kwargs. Additional testing includes: - interactive password prompts tests (rendering coverage exemption pragmas obsolete). - import regression tests for ed25519 and ecdsa keys, using pre-generated keys instead of on-the-fly generated keys. - stronger assertions, e.g. actually check the key length to test the custom bits parameter for RSA key creation, or look at error message contents - more error testing, e.g. by trying to load invalid keys, or keys Additionally, identifies and adds tests + FIXME comments for unexpected behavior of the interface: - import_ed25519_publickey_from_file can import private keys - import_ed25519_privatekey_from_file can import public keys - import_ecdsa_publickey_from_file can import ed25519 public keys
Recent test additions these test coverage exemptions obsolete.
Wow, we lost all our coverage. I assume this is a misconfig or a bug... |
Coverage is honestly one of the well-intentioned but mostly-misguided things I've seen in testing... |
I think that was caused by a force-push interrupting a build.
I think it's not that bad. :) But I agree that one should not overrate coverage results, especially if developers are very liberal with coverage exemptions, some of which I'm removing in this PR. Btw. the build is still failing because of coverage dropping below 99% on Python 3.5. Looking at the results, it seems like it's only a rounding difference, because the absolute coverage is the same as on other versions. Anyways, I think 99% is a rather strict threshold. If you don't mind I'll lower it a bit. |
Our coverage is currently very close to the strict 99 threshold, and we sometimes go below due to different rounding strategies on different builds (different coverage versions?) or if we add/remove code and the relative coverage ratio changes slightly. This commit lowers the strict coverage threshold from 99 to 97. are very close to that threshold
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like a solid set of enhancements to me, thanks Lukas!
Could you file the FIXME comments as issues?
Fixes: # -
Description of the changes being introduced by the pull request:
Note, this PR does not change the interface behavior, it only cleans up the test module, with the goal to make future interface changes more easily comprehensible by looking at changes in the test module.
The clean up includes:
Use setUp function instead
one or the other "unit".
Instead aggregate key generation and loading testing in a single
test function per key type (rsa, ed25519, ecdsa).
different combinations of args and kwargs.
This PR also includes additional testing:
pragmas obsolete).
pre-generated keys instead of on-the-fly generated keys.
test the custom bits parameter for RSA key creation, or look at
error message contents
This PR also identifies and adds tests and FIXME comments for unexpected interface behavior, identified while updating the tests, i.e.
Please verify and check that the pull request fulfils the following requirements: